Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add "bitmapfilter" #8786

Merged
merged 37 commits into from
Jan 22, 2024
Merged

Add "bitmapfilter" #8786

merged 37 commits into from
Jan 22, 2024

Conversation

jepler
Copy link
Member

@jepler jepler commented Jan 5, 2024

bitmapfilter.morph is taken from openmv's imlib.

It is substantially faster than blur/sharpen implemented in ulab, by up to 10x. It also avoids making many allocations.

@jepler
Copy link
Member Author

jepler commented Jan 5, 2024

Creating as a draft PR in part because if we go this direction I'll want to add more processing algorithms (sharpen, grayscale, sepia, edge detect for example). It also needs docstrings etc.

I chose to make this separate from "bitmaptools" in case of source code growth, other name ideas are welcome.

jepler added 2 commits January 5, 2024 14:16
(there were no uses of port_realloc to fix)
bitmapfilter.morph is taken from openmv's imlib.

It is substantially faster than blur/sharpen implemented in ulab,
by up to 10x. It also avoids making many allocations.
@jepler
Copy link
Member Author

jepler commented Jan 5, 2024

I'll disable it on all boards where it doesn't fit when I next update the branch (approximately 18 boards)

jepler added 25 commits January 6, 2024 13:34
 * weight can be any sequence (& test it)
 * improve error message
 * correct documentation of ``mask`` vs copypaste from openmv
This allows operations between channels in an image. It can be used for
the following use cases:
 * Conversion to B&W or sepia
 * Adding color casts
 * Mixing or swapping arbitrary channels
 * Inverting or scaling arbitrary channels
we want it for memento, and maybe for qualia. we can add it elsewhere
later if we want
These boards all previously had camera disabled too.
morph9 is a form of morph which performs 9 different convolutions,
like a version of mix where each coefficient is a (2n+1)x(2n+1) matrix.

Most use cases are covered by morph-then-mix, but some advanced operations
may be more efficient to implement via morph9.
This reduces the time from about 133ms to about 122ms on my test
image on the memento pycamera

a similar change to morph did not produce a performance improvement,
so I didn't include it.
.. and no specific use case for morph9 is known that can't be done
with mix+morph.

Saves ~1800 bytes on Memento
@jepler jepler marked this pull request as ready for review January 16, 2024 21:04
@jepler jepler requested a review from tannewt January 16, 2024 22:08
Copy link
Member

@tannewt tannewt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good overall! Just a couple questions. No major concerns about the division between bindings and implementation.

shared-bindings/bitmapfilter/__init__.c Outdated Show resolved Hide resolved
shared-bindings/bitmapfilter/__init__.c Outdated Show resolved Hide resolved
shared-bindings/bitmapfilter/__init__.c Outdated Show resolved Hide resolved
shared-bindings/bitmapfilter/__init__.c Show resolved Hide resolved
@jepler jepler requested a review from tannewt January 18, 2024 17:24
Copy link
Member

@tannewt tannewt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a couple more things. Thanks!

shared-bindings/bitmapfilter/__init__.c Outdated Show resolved Hide resolved
shared-bindings/bitmapfilter/__init__.c Show resolved Hide resolved
@jepler jepler requested a review from tannewt January 18, 2024 20:39
Copy link
Member

@tannewt tannewt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple typos and one suggestion. Thanks!

//| mixed into the red output channel. The next 3 numbers are for
//| green, and the final 3 are for blue.
//|
//| If ``weights`` `ChannelMixerOffset`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
//| If ``weights`` `ChannelMixerOffset`
//| If ``weights`` is a `ChannelMixerOffset`

Comment on lines 312 to 315
//| object, then each channel is scaled and offset independently:
//| The first two numbers are applied to the red channel: scale and
//| offset. The second two number are applied to the green channel,
//| and the last two numbers to the blue channel.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of explaining what number in each position does, it would be better to give examples when it is useful. The tuple has the individual number doc.

MP_DEFINE_CONST_FUN_OBJ_KW(bitmapfilter_mix_obj, 0, bitmapfilter_mix);

//| def solarize(bitmap, threshold: float = 0.5, mask: displayio.Bitmap | None = None):
//| """Creat a "solarization" effect on an image
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
//| """Creat a "solarization" effect on an image
//| """Create a "solarization" effect on an image

.. and update the test accordingly, fixing a bug discovered in the
process.
@jepler jepler requested a review from tannewt January 18, 2024 23:42
Copy link
Member

@tannewt tannewt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! Well explained.

@jepler jepler merged commit 6837e4f into adafruit:main Jan 22, 2024
229 checks passed
@jepler jepler deleted the bitmapfilter branch January 22, 2024 14:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants